-
Notifications
You must be signed in to change notification settings - Fork 920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[wgpu-core] Return submission index for map_async
and on_submitted_work_done
to track down completion of async callbacks
#6360
Conversation
I looked a tiny little bit into this and I think the approach with just looking at whatever the active or last successful submission index is not going to work well:
The second bullet points means that the mapping has to be available at the very latest point when all active submissions at the time of calling Another high level issue that hasn't been adressed here so far (and wtf why is the CI not tripping on this 😱 ) is that by exposing this as part of the |
Thanks for taking the time to have a look! 🙏
You're not missing anything, this +1 was added out of my doubt about how submission indices must be interpreted, I wanted to avoid reading the mapped buffer too early so conservativatively that I end up doing it too late! I've read the code of the
If by "WebGPU" you mean the JavaScript API, this is supposedly exposed as a Promise object. I do not know how Firefox exactly uses |
I'd need to dig in deeper again, but it tracks which buffers are waiting for mapping.
No, that's not what I meant: The wgpu crate is an interface to either a host implementing WebGPU or an implementation of WebGPU (confusingly, one of them based on WebGL!). The former is found here, the later (that's what you adjusted so far) is here. Since wgpu is supposed to have a consistent api across both, we make those promises be a callback under all circumstances. |
Ow, I did not remember that! I get it now. And that explains part of my confusion. I'll try and rework this PR in the light of this, thx! |
Okey I've tried adding a |
frankly at this point I don't know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
map_async
and on_submitted_work_done
to track down completion of async callbacks
Problem
wgpu-native is upgrading to latest versions of
webgpu-headers
(see gfx-rs/wgpu-native#427), and a notable change since the last sync is the introduction ofWGPUFuture
. Such structure holds au64
, from which we must be able to tell whether the async operation has completed.A good candidate for the payload of
WGPUFuture
is the submission index already used in some places (andu64::MAX
to mean "a future that is already resolved, probably because submission failed"). The only issue is that submit operations do not necessarily return a submission index.Description
This PR makes functions
buffer_map_async
,add_work_done_closure
,map_async
,queue_on_submitted_work_done
return the submission index that can later be used to check that the operation completed.Testing
This is a WIP, I need feedback on the use of
last_successful_submission_index
inqueue_on_submitted_work_done
(is this the right fallback?), feedback about how to check that an operation completed from its submission index, and there is something to discuss aboutmap_async
:active_submission_index
to make sure the operation is not considered complete too early, but maybe this can have the callback be invoked with some delay if the buffer mapping operation was not the last in queue?map_async
returns, we do not know yet the true submission index of the map operation (it may be lower than theactive_submission_index
, right? If not then the proposed solution is ok, otherwise should we trigger triage right away? I guess there is a good reason why it is done separately though.